Skip to content

[ONEMKL_SYCL][REF] Add operation_info_t and matrix_opt to more cases and add two examples to cover it#70

Merged
spencerpatty merged 6 commits intomainfrom
dev/spatty/add_inspect_onemkl_sycl
Apr 13, 2026
Merged

[ONEMKL_SYCL][REF] Add operation_info_t and matrix_opt to more cases and add two examples to cover it#70
spencerpatty merged 6 commits intomainfrom
dev/spatty/add_inspect_onemkl_sycl

Conversation

@spencerpatty
Copy link
Copy Markdown
Contributor

@spencerpatty spencerpatty commented Mar 27, 2026 •
edited
Loading

Summary:
1. add spmm_csr and sptrsv_csr examples with more complete API usage models (using operation_info_t and inspect functions)
2. add xyz_inspect and operation_info_t, and matrix_opt to ONEMKL_SYCL backend
3. add several more variants of reference function with more complete set of apis

Merge Checklist:

  • Passing CI
  • Update documentation or README.md
  • Additional Test/example added (if applicable) and passing
  • At least one reviewer approval
  • (optional) Clang sanitizer scan run and triaged
  • Clang formatter applied (verified as part of passing CI)

@spencerpatty
expand reference and onemkl_sycl backend to use operation_info_t, and…
c55e179 
… introduce xyz_inspect for many cases, also add two new examples which uses them
@spencerpatty spencerpatty self-assigned this Mar 27, 2026
@spencerpatty spencerpatty added the enhancement New feature or request label Mar 27, 2026
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/detail/create_matrix_handle.hpp
Comment on lines +43 to +47
#if defined(__INTEL_MKL__) && \
((__INTEL_MKL__ == 2025) && (__INTEL_MKL_MINOR__ == 3) || \
(__INTEL_MKL__ > 2025))
m.size(), // nnz added in 2025.3, and without deprecated
#endif
Copy link
Copy Markdown
Contributor Author

@spencerpatty spencerpatty Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MKL SYCL api change in 2025.3 adding nnz to csr/csc/bsr and adding coo format which already had nnz in definition.

spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spgemm_impl.hpp
Comment on lines +146 to +152
info.update_impl_(tmp_info.result_shape(), tmp_info.result_nnz());
info.state_.a_handle = tmp_info.state_.a_handle;
info.state_.b_handle = tmp_info.state_.b_handle;
info.state_.c_handle = tmp_info.state_.c_handle;
info.state_.descr = tmp_info.state_.descr;
info.state_.c_rowptr = tmp_info.state_.c_rowptr;
info.state_.q = tmp_info.state_.q;
Copy link
Copy Markdown
Contributor Author

@spencerpatty spencerpatty Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe could utilize a copy constructor here ?

Copy link
Copy Markdown
Contributor

@yhmtsai yhmtsai Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should info_t be copyable?

Copy link
Copy Markdown
Contributor Author

@spencerpatty spencerpatty Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess the answer is no iot should not be copyable at all, since workspaces or arrays are not generally shareable, but we can do shallow copies like this by hand as we desire.

spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spmm_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spmm_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spmm_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spmm_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spmv_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spgemm_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spgemm_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spmm_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spmm_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spmm_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spmm_impl.hpp
spencerpatty
spencerpatty commented Mar 27, 2026
View reviewed changes
Comment thread include/spblas/vendor/onemkl_sycl/spmv_impl.hpp
@spencerpatty
Add log_trace("") to many of the new apis for future debugging
cd88422 
Co-authored-by: Spencer Patty <spencer.patty@intel.com>
yhmtsai
yhmtsai requested changes Apr 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@yhmtsai yhmtsai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only some nit. Others lgtm.

Comment thread examples/CMakeLists.txt Outdated
Comment thread examples/sptrsv_csr.cpp
Comment thread examples/spmm_csr.cpp Outdated
Comment thread include/spblas/algorithms/multiply_impl.hpp Outdated
Comment thread include/spblas/vendor/onemkl_sycl/spgemm_impl.hpp
Comment on lines +146 to +152
info.update_impl_(tmp_info.result_shape(), tmp_info.result_nnz());
info.state_.a_handle = tmp_info.state_.a_handle;
info.state_.b_handle = tmp_info.state_.b_handle;
info.state_.c_handle = tmp_info.state_.c_handle;
info.state_.descr = tmp_info.state_.descr;
info.state_.c_rowptr = tmp_info.state_.c_rowptr;
info.state_.q = tmp_info.state_.q;
Copy link
Copy Markdown
Contributor

@yhmtsai yhmtsai Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should info_t be copyable?

@yhmtsai yhmtsai mentioned this pull request Apr 9, 2026
Open
12 tasks
spencerpatty and others added 3 commits April 10, 2026 16:21
@spencerpatty @yhmtsai
Update examples/CMakeLists.txt
669b8d7 
Co-authored-by: Yu-Hsiang M. Tsai <19565938+yhmtsai@users.noreply.github.com>
@spencerpatty @yhmtsai
Update examples/spmm_csr.cpp
8632f30 
Co-authored-by: Yu-Hsiang M. Tsai <19565938+yhmtsai@users.noreply.github.com>
@spencerpatty @yhmtsai
Update include/spblas/algorithms/multiply_impl.hpp
1515c9c 
Co-authored-by: Yu-Hsiang M. Tsai <19565938+yhmtsai@users.noreply.github.com>
@spencerpatty spencerpatty requested a review from yhmtsai April 10, 2026 23:43
@spencerpatty
Copy link
Copy Markdown
Contributor Author

spencerpatty commented Apr 10, 2026

Hi Mike, I made the few changes, and it requires another review for approval.

yhmtsai
yhmtsai approved these changes Apr 11, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@yhmtsai yhmtsai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@spencerpatty spencerpatty merged commit 01cfd9f into main Apr 13, 2026
10 of 12 checks passed
@spencerpatty spencerpatty deleted the dev/spatty/add_inspect_onemkl_sycl branch April 13, 2026 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yhmtsai yhmtsai yhmtsai approved these changes

@BenBrock BenBrock Awaiting requested review from BenBrock

Assignees

@spencerpatty spencerpatty

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@spencerpatty @yhmtsai